Skip to content

test: add OpenTelemetry context validation test in Logging client - #14461

Draft
jinseopkim0 wants to merge 3 commits into
mainfrom
test-logging-client-validation
Draft

jinseopkim0 wants to merge 3 commits into
mainfrom
test-logging-client-validation

Conversation

@jinseopkim0

Copy link
Copy Markdown
Contributor

Validate that Logging client auto-populates trace metadata when an OpenTelemetry span is active in context.

Fixes b/564460092

@jinseopkim0

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a cleanup step for context priority in the test teardown and introduces a new test to verify metadata auto-population with an OpenTelemetry context. The review feedback suggests improving code readability in the new test by importing the OpenTelemetry classes instead of using their fully qualified names.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a teardown cleanup step for the context priority and introduces a new test to verify metadata auto-population with OpenTelemetry. The feedback suggests managing the OpenTelemetrySdk instance with a try-with-resources block to prevent resource leaks and importing OpenTelemetry classes instead of using fully qualified names to improve readability.

@jinseopkim0
jinseopkim0 force-pushed the test-logging-client-validation branch from 16f7907 to 226a3cd Compare September 21, 2026 16:45
@jinseopkim0

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@jinseopkim0

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a new unit test testAutoPopulationWithOpenTelemetryContext to AutoPopulateMetadataTests.java to verify that log entry metadata (trace, span ID, and trace sampled) is correctly auto-populated when an OpenTelemetry context is active. It also updates the teardown method to clean up the current context priority. There are no review comments, and I have no additional feedback to provide.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a new unit test to verify metadata auto-population when an OpenTelemetry context is active, and updates the teardown logic to clear the context priority. The review feedback suggests simplifying the test setup by removing the unused InMemorySpanExporter and SpanProcessor along with their corresponding imports, as they are not needed to verify the propagation of the active SpanContext.

@jinseopkim0

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates AutoPopulateMetadataTests.java to add a new test verifying the auto-population of metadata when using an OpenTelemetry context. It imports the necessary OpenTelemetry classes, ensures the current context priority is removed during teardown, and implements testAutoPopulationWithOpenTelemetryContext to validate trace, span ID, and sampling status. There are no review comments, and we have no feedback to provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant